Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Consolidate discovery options for all providers and remove duplicate all/auto from CLI help info. #4840

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

VasilSirakov
Copy link
Contributor

@VasilSirakov VasilSirakov commented Nov 11, 2024

Fixes mondoohq/cnspec#1473

In this PR:

  • Keep (all/auto) as default for all providers.
  • Ensure every provider specifies its supported discovery options in config.go (except all/auto).
  • Removed some unused code.

Copy link
Contributor

github-actions bot commented Nov 11, 2024

Test Results

3 145 tests  ±0   3 144 ✅ ±0   1m 23s ⏱️ -3s
  371 suites ±0       1 💤 ±0 
   28 files   ±0       0 ❌ ±0 

Results for commit e9cf94f. ± Comparison against base commit d48a9a3.

♻️ This comment has been updated with latest results.

@VasilSirakov VasilSirakov marked this pull request as ready for review November 11, 2024 12:34
@VasilSirakov
Copy link
Contributor Author

As per @imilchev remarks, we should keep all and auto for all providers by default and simply remove in config.go the occurrences where individual providers specify DiscoveryAll and DiscoveryAuto to avoid having duplicates.

@VasilSirakov VasilSirakov force-pushed the vasil/consolidate-discovery-opts branch 2 times, most recently from aaa666d to a06f282 Compare November 14, 2024 06:32
…all/auto options from CLI help.

Signed-off-by: Vasil Sirakov <[email protected]>
Signed-off-by: Vasil Sirakov <[email protected]>
Signed-off-by: Vasil Sirakov <[email protected]>
Signed-off-by: Vasil Sirakov <[email protected]>
@VasilSirakov VasilSirakov force-pushed the vasil/consolidate-discovery-opts branch from ee822b9 to e9cf94f Compare November 18, 2024 07:33
@VasilSirakov VasilSirakov merged commit 3310081 into main Nov 18, 2024
16 checks passed
@VasilSirakov VasilSirakov deleted the vasil/consolidate-discovery-opts branch November 18, 2024 08:29
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cnspec discover option all,auto twice
2 participants